suppress: avoid orphaning foreign linter pragmas#3454
Conversation
|
Hi @nitishagar! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Hi @nitishagar - thanks for the PR! We can review it (and any other PRs from you) as soon as you sign the CLA (see the instructions here). |
|
@rchen152 I've signed the CLA. Let me know if you have any questions or anything I can clarify on the above changes. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Thanks! @yangdanny97 would you mind taking a look at this one, since it looks like you had started thinking about #3373? |
When the line above a target already contains a non-pyrefly linter pragma (noqa, nosemgrep, pyright:, pylint:, etc.), `pyrefly suppress` was inserting a `# pyrefly: ignore [...]` line between them, which silently disabled the foreign pragma. Detect that case and append inline at the end of the target line instead. Fixes facebook#3373
nitishagar
force-pushed
the
nitishagar/issue-3373-suppress-pragma-conflict
branch
from
ef4617a to
c0f67a5
Compare
|
Rebased onto latest main and resolved the conflicts (Cargo.lock + suppress.rs) — ready for review whenever convenient. Thanks! |
3 participants
Summary
pyrefly suppress(defaultLineBeforemode) inserts# pyrefly: ignore [...]on the line above a target. When the line directly above already contains a non-pyrefly linter pragma —# noqa,# nosemgrep,# pyright: ignore,# pylint:,# nosec,# ruff:,# flake8:, foreign-tool# type: ignore— the new pyrefly comment slides between them, breaking the foreign pragma's adjacency to its target line and silently disabling it.This PR detects that case and appends
# pyrefly: ignore [...]to the end of the target line instead, leaving the foreign pragma untouched. Behavior for plain comments and other situations is unchanged.has_foreign_linter_pragma(line)helper (uses the existingfind_comment_start_in_lineso string literals don't false-match).add_suppressions'sLineBeforebranch via aforce_inlineflag.Fixes #3373
Test plan
cargo test -p pyrefly --lib— 5161/5161 passing locally (includes the 6 new tests).test_add_suppressions_existing_comment(where the line above is a plain comment, not a pragma) still passes — confirms behavior is unchanged for non-pragma comments.